-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Make calls to BM25Scorer#score inlinable. #15082
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I ran experiments locally that suggest that some of the performance decrease from type pollution (mikemccand/luceneutil#436) can be attributed to calls to `SimScorer#score` no longer being inlinable since they are polymorphic. This change helps `BM25Scorer` remain inlinable using similar tricks that we are applying for `Bits#get` and `ImpactsEnum#nextDoc`/`ImpactsEnum#advance`. Hopefully changes such as apache#15039 will help improve performance with other similarities as well in the future.
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
This PR does not have an entry in lucene/CHANGES.txt. Consider adding one. If the PR doesn't need a changelog entry, then add the skip-changelog label to it and you will stop receiving this reminder on future updates to the PR. |
Why not just commit #15039 instead of this, which will also solve this problem and make it bimorphic? |
I was hesitating which one to commit first. #15039 helps less in my benchmarks for now, likely because it only helps the first clause of conjunctive queries, because it doesn't help with computing impact scores, and because it only helps when scores are computed via the BulkScorer API instead of Scorer API (so e.g. AndHighOrMedMed doesn't see a speedup). I can start with #15039 plus follow-ups and see how much this PR still helps when I get stuck. |
But this PR is also incomplete and ooesn't hook in the logic for things like SynonymQuery, PhraseQuery, which led to my confusion :) |
This PR has not had activity in the past 2 weeks, labeling it as stale. If the PR is waiting for review, notify the [email protected] list. Thank you for your contribution! |
I'm closing: The main queries that this change now helps are |
I ran experiments locally that suggest that some of the performance decrease from type pollution (mikemccand/luceneutil#436) can be attributed to calls to
SimScorer#score
no longer being inlinable since they are polymorphic. This change helpsBM25Scorer
remain inlinable using similar tricks that we are applying forBits#get
andImpactsEnum#nextDoc
/ImpactsEnum#advance
.Hopefully changes such as #15039 will help improve performance with other similarities as well in the future.